-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: update gitlab repo remote url #354
Conversation
Signed-off-by: Trong Nhan Mai <[email protected]>
…ith local path Signed-off-by: Trong Nhan Mai <[email protected]>
src/macaron/errors.py
Outdated
@@ -30,3 +30,7 @@ class ConfigurationError(MacaronError): | |||
|
|||
class CloneError(MacaronError): | |||
"""Happens when cannot clone a git repository.""" | |||
|
|||
|
|||
class RepoError(MacaronError): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The relation between a RepoError
and CloneError
is not clear and they seem to have an overlap. RepoError
seems to be related to checking out a repo, so I wonder if it should be renamed to something like CheckOutError
? Or we could add a RepoError
as a parent class of CloneError
and CheckOutError
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will rename RepoError
to RepoCheckOutError
and update the doc string accordingly for clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 38db651
@@ -504,6 +504,7 @@ def _prepare_repo( | |||
Git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type here should be Git | None
. Can you please fix it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 204d5f8.
RepoError | ||
If there is an error while checking out the specific branch or commit. | ||
""" | ||
raise NotImplementedError() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need to raise the NotImplementedError
. The abc
module handles unimplemented abstract methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 73cd1bc
Always raise, since this method should not be used to check out in any repository. | ||
""" | ||
raise RepoError( | ||
f"Internal error when checking out branch {branch} and commit {digest} for repo {git_obj.project_name}." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of Internal error
can you please be more specific. For example here the error should communicate that we cannot check out from an empty git service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 77e4d14.
git_url.clone_remote_repo(clone_dir, clone_url) | ||
repo = git_url.clone_remote_repo(clone_dir, clone_url) | ||
|
||
# If ``git_url.clone_remote_repo`` return an Repo instance, this means that the repository is freshly cloned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# If ``git_url.clone_remote_repo`` return an Repo instance, this means that the repository is freshly cloned | |
# If ``git_url.clone_remote_repo`` returns a Repo instance, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 6678fe2
try: | ||
reconstructed_url = self.construct_clone_url(remote_origin_url) | ||
except CloneError as error: | ||
raise RepoError("Internal error prevent preparing the repo for the analysis.") from error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about being more specific about the error and avoiding Internal error
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 77e4d14
Signed-off-by: Trong Nhan Mai <[email protected]>
Signed-off-by: Trong Nhan Mai <[email protected]>
Signed-off-by: Trong Nhan Mai <[email protected]>
Signed-off-by: Trong Nhan Mai <[email protected]>
Signed-off-by: Trong Nhan Mai <[email protected]>
Signed-off-by: Trong Nhan Mai <[email protected]>
This PR does the following tasks:
TODO: